Skip to content

fix(control): clear completed JTT state and remove idle holding - #3969

Merged
mustafab0 merged 3 commits into
mainfrom
cc/fix/jtt-preempt
Sep 8, 2026
Merged

fix(control): clear completed JTT state and remove idle holding#3969
mustafab0 merged 3 commits into
mainfrom
cc/fix/jtt-preempt

Conversation

@TomCC7

@TomCC7 TomCC7 commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Contribution path

Fixes a regression introduced in #3610.

Problem

  • JTT reused stale commanded positions after another task moved the joints, causing dangerous jumps on the next execution.
  • Idle holding duplicated the shared hardware interface's handling of retained joint targets.

Solution

  • Clear each joint's commanded state after its final output; initialize new executions from measured positions.
  • Keep commanded-state continuity during active execution and trajectory replacement.
  • Remove hold_position_when_idle and its G1 configuration.
  • Add regression tests verifying JTT does not pull joints back after another task moves them.

How to Test

  • Manually verified on hardware: teleop followed by trajectory execution no longer produces the dangerous motion.
  • Verified trajectory completion and teleop handoff in G1 MuJoCo simulation.
  • 128 unit tests passed:
uv run pytest dimos/control/tasks/trajectory_task/test_trajectory_task.py dimos/control/test_control.py dimos/control/test_coordinator_routing.py -q

AI assistance

Codex (GPT-6): diagnosis, implementation, tests, and PR description.

Checklist

  • I have read and approved the CLA.

@codecov

codecov Bot commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.48485% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...trol/tasks/trajectory_task/test_trajectory_task.py 98.30% 0 Missing and 1 partial ⚠️ 
@@            Coverage Diff             @@
##             main    #3969      +/-   ##
==========================================
- Coverage   77.91%   77.91%   -0.01%     
==========================================
  Files        1398     1399       +1     
  Lines      132268   132293      +25     
  Branches    11487    11484       -3     
==========================================
+ Hits       103061   103079      +18     
- Misses      25943    25949       +6     
- Partials     3264     3265       +1
Components Coverage Δ
Tests 94.72% <98.30%> (-0.01%) ⬇️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.26% <98.48%> (+<0.01%) ⬆️
OS-ubuntu-latest 74.76% <98.48%> (+<0.01%) ⬆️
Py-3.10 74.76% <98.48%> (+<0.01%) ⬆️
Py-3.11 74.75% <98.48%> (+<0.01%) ⬆️
Py-3.12 74.75% <98.48%> (+<0.01%) ⬆️
Py-3.13 74.76% <98.48%> (+<0.01%) ⬆️
Py-3.14 74.76% <98.48%> (+<0.01%) ⬆️
Py-3.14t 74.76% <98.48%> (+<0.01%) ⬆️
SelfHosted-Large 30.71% <18.18%> (-0.01%) ⬇️
SelfHosted-Linux 35.76% <19.69%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/control/tasks/trajectory_task/trajectory_task.py 89.20% <100.00%> (+0.51%) ⬆️
dimos/control/test_control.py 96.74% <ø> (-0.10%) ⬇️
...nitree/g1/blueprints/basic/unitree_g1_groot_wbc.py 58.33% <100.00%> (+0.34%) ⬆️
...trol/tasks/trajectory_task/test_trajectory_task.py 98.30% <98.30%> (ø)

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TomCC7 TomCC7 changed the title fix(control): clear completed JTT commands before teleop handoff fix(control): clear completed JTT state and remove idle holding Sep 8, 2026
@TomCC7
TomCC7 marked this pull request as ready for review September 8, 2026 06:50
@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

No merge-blocking issue was established in the changed trajectory control logic.

T-Rex validation blocked

The focused trajectory lifecycle check and narrow control suite could not reach the changed code because the temporary environment was missing dimos_lcm and pydantic_settings.

Confidence Score: 5/5

No established issue blocks merging.

No severity-bearing findings were established.

Files Needing Attention: No specific file requires follow-up; the focused control suite should be rerun in an environment with the required Python dependencies.

T-Rex T-Rex Logs

What T-Rex did

  • A focused trajectory lifecycle harness, including a trajectory factory, completion, replacement, and preemption, was prepared for before-and-after execution.
  • The base harness import attempt halted during imports because dimos_lcm was unavailable, preventing any control behavior from running.
  • The updated harness import attempt halted during imports for the same dimos_lcm blocker, before pre-execution work occurred.
  • The narrow control suite loading stopped while loading test configuration because pydantic_settings was unavailable, preventing test collection.
  • The harness and validation runner artifacts were uploaded for review, including the harness source and the run-validation script.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "test(control): assert JTT continuity aft..." | Re-trigger Greptile

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 8, 2026
@TomCC7
TomCC7 added this pull request to the merge queue Sep 8, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 8, 2026
@mustafab0
mustafab0 added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 722fb93 Sep 8, 2026
44 of 55 checks passed
@mustafab0
mustafab0 deleted the cc/fix/jtt-preempt branch September 8, 2026 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paul-nechifor paul-nechifor paul-nechifor approved these changes
@leshy leshy Awaiting requested review from leshy leshy is a code owner
@spomichter spomichter Awaiting requested review from spomichter spomichter is a code owner
@mustafab0 mustafab0 Awaiting requested review from mustafab0 mustafab0 is a code owner
@Dreamsorcerer Dreamsorcerer Awaiting requested review from Dreamsorcerer Dreamsorcerer is a code owner
@aclauer aclauer Awaiting requested review from aclauer aclauer is a code owner

Assignees

No one assigned

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TomCC7 @paul-nechifor @mustafab0